• Somewhat unrelated but removed quotes around documentation pages in the script list, e.g. "new_script.gd" becomes new_script.gd, since this resulted in messy truncations of paths (to test, create new_folder/new_script.gd and new_script.gd and open both their documentations on stable.

I think it's better to fix this by checking the tab type. Doc tabs should never truncate the name, only script tabs.

• Somewhat unrelated but removed quotes around documentation pages in the script list, e.g. "new_script.gd" becomes new_script.gd, since this resulted in messy truncations of paths (to test, create new_folder/new_script.gd and new_script.gd and open both their documentations on stable.

I think it's better to fix this by checking the tab type. Doc tabs should never truncate the name, only script tabs.

Why the difference? You can still get the full path by hovering and it's at the top of the help document. This way it feels like behavior is at least consistent?

Works great!! Good job!!

I tested this PR merging locally #95965 and #96002

There's a last problem where the doc loading thread can be reloading the doc script cache while the EditorFileSystem is scanning and updating the doc. That could lead to problems where the cache is loaded after the remove_doc or add_doc was called so the last version of the script is still in the documentation. Worst, we could try to read the cache while trying to delete it and vice versa. I suggest the script cache loading should be postponed after the first scan. You already done the logic when the cache is not present to wait for signal filesystem_changed if is_scanning is true.

Tested:

• Modifying scripts in the builtin Script Editor
• Modifying scripts in a external editor connected to LSP server (ex: VSCode)
• Modifying scripts in a external editor not connected to LSP server (ex: NotePad) (Was not working without Fix script properties reload from external editor #96002. I also found a bug there that I just fixed in the Fix script properties reload from external editor #96002)
• Modifying scripts in a external editor while the doc is opened in Godot (automatically updates with returning to godot)
• Modifying scripts while the editor is closed: Not working perfectly, see comment above.
• Removing scripts while the editor is closed: Not working perfectly, see comment above.

Good news, this prerequisite PR has been merged: #95965

Good news, this prerequisite PR has been merged: #95965

Perfect, rebased, fixed a couple of things and pushed! Been too sleepy/tired to work on this, but I don't know that we have an immediately obvious solution to the problem you mentioned.

If we want to wait to load the cache until the first scan is complete, maybe EditorFileSystem_update_script_documentation() has already called EditorHelp::add/remove_doc, and those changes will be overwritten when the cache is read. I believe the dependencies we're looking at are:

If regenerating the cache: 1) First scan -> 2) Regenerate cache from scratch
If loading existing cache: 1) Load cache -> 2) EditorFileSystem updates script docs

I'm worried about creating a race condition where EditorHelp is waiting for first scan, but before first scan finishes, it waits for EditorHelp to load the cache. Will try to give this some more thought when life allows!

This entire summer i couldn't understand what was the issue with custom Documentations and why i had to resave scripts in order to make them working again.
I'm really hoping your fix will be implemented asap in next updates! Tysm for contibuting into Godot better future!

Been working on this and running into a couple of problems.

If the cache has not been generated, then Godot freezes then runs out of memory & crashes here with a project with 4096 .gd files:

I did rebase onto master today, so I suspect it's from recently introduced changes elsewhere (no significant changes in this PR) . Maybe from the progress bar or something about loading many files. It fails at ~4015 processed files somewhere in a loop. If the cache has been generated and is just loaded, no such issue occurs.

Also, if a script is updated using an external editor while Godot is open, the script editor updates but not the open doc screen. This is true both with the previous (non-rebased & not updated) version of the as well as the new one. I thought it worked before, but doesn't seem to for me.

EditorHelp::add_doc(docs[j]); in EditorFileSystem::_update_script_documentation() contains the old documentation. Adding CACHE_MODE_IGNORE or CACHE_MODE_REPLACE to the corresponding ResourceLoader::load() call in _update_script_documentation() messes with the pop-up asking whether we want to reload from disk or not. Interestingly, _update_script_documentation seems to be called before that choice is made.

I'll keep investigating.

Rebased once more and the crash seems to have been fixed, even in a project wit 8192 files. Will dig into the script updating but not the script doc!

Very very nice modifications that you made on your last commit. I like the way you postponed the cache loading until the first scan is done and that you keep track of the changes while scanning and apply them after the cache is loaded!!!

Also, if a script is updated using an external editor while Godot is open, the script editor updates but not the open doc screen. This is true both with the previous (non-rebased & not updated) version of the as well as the new one. I thought it worked before, but doesn't seem to for me.

The PR #96002 fixes the issue where the doc is not updated in memory, but there's still a issue if the doc is opened in the script editor, it's not updated, the doc needs to be closed and reopened. I'm not sure it's supposed to be fixed in this current PR, I'll look into it in #96002.

Tested:

• Modifying scripts in the built-in Script Editor
• Modifying scripts in a external editor connected to LSP server (ex: VSCode)
• Modifying scripts in a external editor not connected to LSP server (ex: NotePad) (Was not working without Fix script properties reload from external editor #96002, not working completely, see comment above)
• Modifying scripts in a external editor while the doc is opened in Godot
• Modifying scripts while the editor is closed
• Removing scripts while the editor is closed
• Tested with a small project (4-5 scripts)
• Tested with a medium project (1000 scripts)

I have been able to replicate the crash. It happens when

1. the editor is run with --verbose, and
2. the cache is not present and therefore regenerated, force-loading all scripts.

It tries to print one line for each of 8192 resources being loaded onto the console, which I believe causes it to run out of memory.

Tested the following with a 8000+ .gd file project:
No filecache, no doc cache: works ✅
Filecache, no doc cache: works ✅
No filecache, doc cache: works, but does regenerate the doc cache which makes sense ✅
Filecache, doc cache: works ✅

Godot open:

• Change file in notepad, then discard changes: works ✅
• Change file in notepad, then accept changes: works ✅ but the .gd file itself is not updated though the documentation is, bisected to Fix script properties reload from external editor #96002
• Delete file: works ✅
• Add new file: works ✅

Godot closed:

• Change file in notepad: occasionally does not update documentation ⚠️
• Delete file: works
• Add new file: works

I can no longer replicate the issue above, though it happened consistently for a few minutes. Tried with many combinations of different .gd files & docs open or closed in the ScriptEditor.

I thought it might be from simultaneous reads/writes to _docs_to_add, but I don't think that's possible since load_script_doc_cache always waits for the filesystem_changed signal. Maybe if load_script_doc_cache is called after the filesystem scan is done but before EditorFileSystem::_update_script_documentation() is called, since there is a single situation in which the signal is emitted a while later, using call_deferred(SNAME("emit_signal"), "filesystem_changed"); // Update later in EditorFileSystem::update_files?

That said, I'm not sure it's worth adding mutexes and locking for a low probability situation which can be fixed manually relatively easily?

Change file in notepad: occasionally does not update documentation

I think I created some regressions with #96002, it could be caused by that. This PR could maybe fix the intermittent problem. I'm not sure it should be investigated further here. It's being addressed in #97168

That said, I'm not sure it's worth adding mutexes and locking for a low probability situation which can be fixed manually relatively easily?

I'm pretty sure it's not necessary to use mutex because the EditorFileSystem and the ScriptEditor reload resources and scripts in the main thread. Also, the situation you are describing should not happen to often. Worse case scenario, the user resave the script.

Change file in notepad: occasionally does not update documentation

I think I created some regressions with #96002, it could be caused by that. This PR could maybe fix the intermittent problem. I'm not sure it should be investigated further here. It's being addressed in #97168

That said, I'm not sure it's worth adding mutexes and locking for a low probability situation which can be fixed manually relatively easily?

I'm pretty sure it's not necessary to use mutex because the EditorFileSystem and the ScriptEditor reload resources and scripts in the main thread. Also, the situation you are describing should not happen to often. Worse case scenario, the user resave the script.

Updated PR with latest feedback!

And sounds great! I will try to get some time and look into those PRs, though EditorFileSystem is still sutble and confusing to me :)

I think you're right on this PR maybe not being the problem.

These are my attempts to understand the possible issues, just for documentation purposes. The issue I was thinking would happen if EditorHelp::_load_script_doc_cache_thread() was running on worker thread while EditorFileSystem::_update_script_documentation() was running on another thread. The other thread could adds docs to _docs_to_add before script_docs_loaded gets set to true, but after _load_script_doc_cache_thread() is done iterating it. They would thus be ignored.

I followed whether fileystem_changed can be emitted before _update_script_documentation() is done running. It is only called in _process_update_pending(), which occurs in 3 places:

1. In _update_scan_actions(): if it returns true, always emitted after
2. In update_files(): always emitted after, but _process_update_pending() is only called when not scanning.
3. In reimport_files(): always emitted after if it's scanning, and the cache always loads, so it's always emitted.

So we are sure filesystem_changed always emits after _update_script_documentation() has run, and the above issue cannot happen.

But maybe the issue is in 2), where _process_update_pending() is not called if is_scanning(). update_files() is only called in _update_scan_actions(), which should call _process_update_pending(). It can also be called from update_file() (singular), which is called many places. Maybe update_file() is called somewhere during scanning, causing the docs to fail to update? I don't know. It gets called on EditorNode::_resource_saved() for example.